Skip to content

add service account id as option to register agentex agent#332

Merged
alvinkam2001 merged 2 commits intonextfrom
akam/agentex-SA-integration
Apr 30, 2026
Merged

add service account id as option to register agentex agent#332
alvinkam2001 merged 2 commits intonextfrom
akam/agentex-SA-integration

Conversation

@alvinkam2001
Copy link
Copy Markdown

@alvinkam2001 alvinkam2001 commented Apr 27, 2026

Summary

Relaxes the environments.yaml auth-principal validator so agent developers can register an agent under a service account instead of a user identity.

  • _validate_single_environment_config now requires exactly one of user_id or service_account_id in the auth.principal block (mutually exclusive). The previous validator hard-required user_id, which blocked SA registration end-to-end.
  • Updates the docstring example in environment_config.py to show both shapes.

Backwards compatible: existing configs with only user_id continue to validate. Field shape mirrors what agentex-auth (SGPPrincipalContext) and SGP (x-user-id / x-service-account-id headers) already use, so no translation layer is needed between yaml and the wire.

Related PRs (ship together)

Test Plan

  • 4 new unit tests in tests/lib/cli/test_validation.py, all passing via uv run pytest:
    • user-only principal validates (backwards-compat)
    • service_account-only principal validates
    • principal with neither id is rejected with a clear error
    • principal with both ids is rejected with a "not both" error
  • Existing test_environment_config.py (18 tests) still passes.
  • ruff check + ruff format clean.

Greptile Summary

This PR relaxes the environments.yaml auth-principal validator to accept either user_id or service_account_id (mutually exclusive), enabling agents to be registered under a service account identity. Existing configs using only user_id are fully backwards-compatible, and four targeted unit tests cover all four principal combinations.

Confidence Score: 5/5

Safe to merge — change is backwards-compatible, logic is correct, and all four principal scenarios are tested.

The only finding is a P2 style suggestion (missing explanatory comment) that does not affect correctness or runtime behavior. All changed paths are covered by new tests.

No files require special attention.

Important Files Changed

Filename Overview
src/agentex/lib/sdk/config/validation.py Core validator updated: replaces hard-required user_id with a mutually-exclusive user_id XOR service_account_id check; error messages and hints updated accordingly. Logic is correct and backwards-compatible.
src/agentex/lib/sdk/config/environment_config.py Docstring-only change: adds a service_account_id YAML example to get_configs_for_env. No model or validation logic changed.
tests/lib/cli/test_validation.py New test file: 4 unit tests covering the four principal scenarios (user_id only, service_account_id only, neither, both). Coverage is complete for the changed validator path.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[_validate_single_environment_config] --> B{principal has user_id?}
    B -- No --> C{principal has service_account_id?}
    C -- No --> D[❌ ValueError: must contain one of user_id or service_account_id]
    C -- Yes --> E[✅ SA path — skip env-name warning]
    B -- Yes --> F{also has service_account_id?}
    F -- Yes --> G[❌ ValueError: not both]
    F -- No --> H{user_id is a string?}
    H -- Yes --> I{contains env indicator?}
    I -- No --> J[⚠️ logger.warning: missing env indicator in user_id]
    I -- Yes --> K[✅ user_id path OK]
    J --> K
    E --> L[validate helm overrides if present]
    K --> L
Loading

Comments Outside Diff (1)

  1. src/agentex/lib/sdk/config/validation.py, line 239-245 (link)

    P2 Helpful error hint omits service_account_id

    The hint block is triggered whenever "user_id" appears in the error message. Both new error messages ("must contain non-empty 'user_id' or 'service_account_id'" and "must contain only one of 'user_id' or 'service_account_id', not both") contain the string "user_id", so this block fires for service_account_id-related failures too — but the tips text mentions only user_id, which will mislead users configuring a service account principal.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: src/agentex/lib/sdk/config/validation.py
    Line: 239-245
    
    Comment:
    **Helpful error hint omits `service_account_id`**
    
    The hint block is triggered whenever `"user_id"` appears in the error message. Both new error messages (`"must contain non-empty 'user_id' or 'service_account_id'"` and `"must contain only one of 'user_id' or 'service_account_id', not both"`) contain the string `"user_id"`, so this block fires for `service_account_id`-related failures too — but the tips text mentions only `user_id`, which will mislead users configuring a service account principal.
    
    
    
    How can I resolve this? If you propose a fix, please make it concise.

    Fix in Cursor Fix in Claude Code

Fix All in Cursor Fix All in Claude Code

Prompt To Fix All With AI
This is a comment left during a code review.
Path: src/agentex/lib/sdk/config/validation.py
Line: 113-119

Comment:
**Missing comment on intentional skip of env-indicator warning**

The `if isinstance(user_id, str):` block silently skips the environment-name warning when `service_account_id` is used instead. This is the right call (SA IDs are pre-created UUIDs, not free-form strings), but it is non-obvious to a future maintainer who sees a codepath with no validation at all for `service_account_id`. A short inline comment would make the intent clear.

```suggestion
    # Check for environment-specific user_id patterns.
    # This warning is intentionally skipped for service_account_id: SA identifiers
    # are pre-created UUIDs whose names are not expected to encode environment info.
    if isinstance(user_id, str):
        if not any(env_name.lower() in user_id.lower() for env_name in ["dev", "prod", "staging", env_name]):
            logger.warning(
                f"User ID '{user_id}' doesn't contain environment indicator. "
                f"Consider including '{env_name}' in the user_id for clarity."
            )
```

**Rule Used:** Add comments to explain complex or non-obvious log... ([source](https://app.greptile.com/review/custom-context?memory=928586f9-9432-435e-a385-026fa49318a2))

**Learned From**
[scaleapi/scaleapi#126958](https://github.com/scaleapi/scaleapi/pull/126958)

How can I resolve this? If you propose a fix, please make it concise.

Reviews (2): Last reviewed commit: "better error message" | Re-trigger Greptile

Context used:

  • Rule used - Add comments to explain complex or non-obvious log... (source)

Learned From
scaleapi/scaleapi#126958

@declan-scale declan-scale changed the base branch from main to next April 27, 2026 19:54
@declan-scale
Copy link
Copy Markdown
Contributor

Updated the base to next, in this package we should always pr against next

@stainless-app stainless-app Bot force-pushed the next branch 2 times, most recently from 16ab771 to f17ad00 Compare April 30, 2026 21:24
@alvinkam2001 alvinkam2001 merged commit a0c443e into next Apr 30, 2026
32 checks passed
@alvinkam2001 alvinkam2001 deleted the akam/agentex-SA-integration branch April 30, 2026 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants